Skip to content

#6276 Improve message parsing for signed message - #6277

Merged
sosnovsky merged 4 commits into
masterfrom
issue-6276-fix-recursive-parsing-issue
Aug 25, 2026
Merged

#6276 Improve message parsing for signed message#6277
sosnovsky merged 4 commits into
masterfrom
issue-6276-fix-recursive-parsing-issue

Conversation

@martgil

@martgil martgil commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

This PR improve message parsing for signed message and prevents recursive parsing.

close #6276


Tests (delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@martgil
martgil requested a review from sosnovsky as a code owner August 21, 2026 04:41
@martgil

martgil commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @sosnovsky - This one is ready for a review. Thanks!

@sosnovsky sosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works well, just a couple minor improvements 👍

Comment thread extension/js/common/core/mime.ts Outdated
Comment thread extension/js/common/core/mime.ts Outdated
type SendingType = 'to' | 'cc' | 'bcc';

export class Mime {
private static readonly MAX_SIGNED_CONTENT_DEPTH = 50;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is similar recursive call in

if (!sigResult?.match && signedContentInDecryptedData) {

which can cause extension lag for nested signed content. we'll need to implement similar fix there, but let's leave it for separate PR

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it - Will remember this one and fix it on separate PR after this one.

@martgil
martgil marked this pull request as draft August 21, 2026 12:10
@martgil
martgil requested a review from sosnovsky August 24, 2026 04:50
@martgil
martgil marked this pull request as ready for review August 24, 2026 04:50
@martgil

This comment was marked as outdated.

Comment thread extension/js/common/core/mime.ts Outdated
Comment on lines +411 to +415
const currentSubject = current.headers.subject?.[0]?.value;
if (!Mime.isSignedContentNode(current)) {
return currentSubject || fallbackSubject;
}
fallbackSubject = currentSubject || fallbackSubject;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's possible for multipart/mixed content to include both signed and unsigned parts. current code stops when it encounters a non-signed node, so it can miss a nested signed node. let’s perform search for another signed node in this case:

const currentSubject = current.headers.subject?.[0]?.value;
fallbackSubject = currentSubject || fallbackSubject;
if (!Mime.isSignedContentNode(current)) {
    const nestedSignedContentNode = Mime.retrieveSignedContentNode([current]);
    if (!nestedSignedContentNode) {
          return fallbackSubject;
    }
    current = nestedSignedContentNode;
    continue;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize, I missed that part. I have rechecked it again and fix the said behavior.

@martgil
martgil requested a review from sosnovsky August 25, 2026 03:46
@martgil

martgil commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @sosnovsky - This one is ready for a review. Thanks!

@sosnovsky sosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all good now, thanks!

@sosnovsky
sosnovsky merged commit 14d1f69 into master Aug 25, 2026
12 checks passed
@sosnovsky
sosnovsky deleted the issue-6276-fix-recursive-parsing-issue branch August 25, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Client slowdown when parsing recursive multipart/signed messages

2 participants